Replace Collectors.toUnmodifiableList() with Stream#toList() #2948
Conversation
[JDK 16 (in JDK-8256441)](https://bugs.openjdk.org/browse/JDK-8256441) added [`Stream#toList()`](https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/util/stream/Stream.html#toList()) provides an optimized version that uses JDK internals to avoid some of the additional array copies that make stream collection expensive.
Generate changelog in
|
| " List<String> f0(Stream<String> in) {", | ||
| " return in.toList();", | ||
| " }", | ||
| // Collectors.toList() supports nulls & is mutable while Stream#toList() does not |
There was a problem hiding this comment.
I think Stream#toList() is meant to allow nulls (based on a test this seems to be the case, and the javadoc suggests implementations should be equivalent to Collections.unmodifiableList(new ArrayList<>(Arrays.asList(this.toArray()))).
Unlike Collectors.toList(), Collectors.toUnmodifiableList() doesn't allow nulls, so this should be a safe replacement, but I think this comment is a bit off.
Confusingly List.of produces an immutable list which doesn't allow nulls, so the newer apis are all over the place wrt nullability...
|
It's not always possible to replace // Stream.collect
<R, A> R collect(Collector<? super T, A, R> collector)
// Collectors.toUnmodifiableList
Collector<T, ?, List<T>> toUnmodifiableList()
// Stream.toList
List<T> toList()So if I have a This is a not uncommon pattern that I've hit a couple times. For example: List<Foo> foos = arguments.stream()
.map(args -> new FooImpl(args))
// This returns List<FooImpl>, not List<Foo>
.toList();You can work around this by explicitly declaring the List<Foo> foos = arguments.stream()
.<Foo>map(args -> new FooImpl(args))
.toList(); |
|
I think we can compare the generic component of the (unverified) |
Before this PR
Uses of
stream.collect(Collectors.toUnmodifiableList())could be replaced withstream.toList()added in JDK 16 and provides a more efficient lower allocation implementation that returns an immutableListthat does not containnullelements.Similar to #2946
After this PR
==COMMIT_MSG==
JDK 16 (in JDK-8256441) added
Stream#toList()provides an optimized version that uses JDK internals to avoid some of the additional array copies that make stream collection expensive.==COMMIT_MSG==
Possible downsides?
This error-prone check does not modify usages of
Collectors.toList()as that returns a modifiableListthat may containnullelements. It also does not modify usages ofImmutableList.toImmutableList()as some use sites leverage GuavaImmutableListtypes to avoid copies viaImmutableList.copyOf(someImmutableList).